Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

skip setproctitle in task_runner on Mac OS #45124

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

skip setproctitle in task_runner on Mac OS #45124

wants to merge 2 commits into from
+17 −5

Conversation

jaketf
Copy link
Contributor

@jaketf jaketf commented Dec 20, 2024

On some newer versions of Mac OS setproctitle can cause segfault benoitc/gunicorn#3021

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@jaketf
skip setproctitle in task_runner on Mac OS
67199f6 
On some newer versions of Mac OS setproctitle can cause segfault
benoitc/gunicorn#3021
ashb
ashb approved these changes Dec 20, 2024
View reviewed changes
Copy link
Member

@ashb ashb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I've started noticed this causing tests on OSX to fail on occassion, but hadn't noticed any runtime issues. Makes sense though.

There are probably a few other cases where we setproctitle (in the dag parser code I just landed inside airflow/dag_processor/ could you update those too?)

jlaneve
jlaneve reviewed Dec 20, 2024
View reviewed changes
airflow/dag_processing/manager.py
Comment on lines +188 to +190
else:
from setproctitle import setproctitle
setproctitle("airflow scheduler -- DagFileProcessorManager")
Copy link
Contributor

@jlaneve jlaneve Dec 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
else:
from setproctitle import setproctitle
setproctitle("airflow scheduler -- DagFileProcessorManager")
else:
from setproctitle import setproctitle
setproctitle("airflow scheduler -- DagFileProcessorManager")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this may actually be a GitHub bug, in the "Files changed" tab it shows the indentation being off, but in the conversation / timeline it shows the indentation as being correct (and my suggested change is unneeded indentation)

Screenshot 2024-12-20 at 6 01 53 PM

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am in contact with setproctitle maintainer during the "Airflow Beach Cleaning" project. I can ask him to comment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After short discussion with @dvarrazzo - it's likely this dvarrazzo/py-setproctitle#144 is going to fix it (unreleased yet).

It would be great though to get some more details about those segfaults @jaketf @ashb when you see them happening again ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pytest task_sdk (locally, not breeze) would trigger it about 10-25% of the time.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

task_sdk/tests/execution_time/test_supervisor.py::TestWatchedSubprocess::test_supervise_handles_deferred_task Fatal Python error: Segmentation fault

Current thread 0x00000001fb058f40 (most recent call first):
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/setproctitle/__init__.py", line 57 in <module>
  File "<frozen importlib._bootstrap>", line 488 in _call_with_frames_removed
  File "<frozen importlib._bootstrap_external>", line 995 in exec_module
  File "<frozen importlib._bootstrap>", line 935 in _load_unlocked
  File "<frozen importlib._bootstrap>", line 1331 in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 1360 in _find_and_load
  File "/Users/ash/code/airflow/airflow/task_sdk/src/airflow/sdk/execution_time/task_runner.py", line 217 in startup
  File "/Users/ash/code/airflow/airflow/task_sdk/src/airflow/sdk/execution_time/task_runner.py", line 339 in main
  File "/Users/ash/code/airflow/airflow/task_sdk/src/airflow/sdk/execution_time/supervisor.py", line 135 in _subprocess_main
  File "/Users/ash/code/airflow/airflow/task_sdk/src/airflow/sdk/execution_time/supervisor.py", line 256 in _fork_main
  File "/Users/ash/code/airflow/airflow/task_sdk/src/airflow/sdk/execution_time/supervisor.py", line 349 in start
  File "/Users/ash/code/airflow/airflow/task_sdk/src/airflow/sdk/execution_time/supervisor.py", line 899 in supervise
  File "/Users/ash/code/airflow/airflow/task_sdk/tests/execution_time/test_supervisor.py", line 295 in test_supervise_handles_deferred_task
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/_pytest/python.py", line 159 in pytest_pyfunc_call
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/pluggy/_callers.py", line 103 in _multicall
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/pluggy/_manager.py", line 120 in _hookexec
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/pluggy/_hooks.py", line 513 in __call__
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/_pytest/python.py", line 1627 in runtest
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/_pytest/runner.py", line 174 in pytest_runtest_call
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/pluggy/_callers.py", line 103 in _multicall
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/pluggy/_manager.py", line 120 in _hookexec
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/pluggy/_hooks.py", line 513 in __call__
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/_pytest/runner.py", line 242 in <lambda>
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/_pytest/runner.py", line 341 in from_call
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/_pytest/runner.py", line 241 in call_and_report
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/_pytest/runner.py", line 132 in runtestprotocol
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/_pytest/runner.py", line 113 in pytest_runtest_protocol
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/pluggy/_callers.py", line 103 in _multicall
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/pluggy/_manager.py", line 120 in _hookexec
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/pluggy/_hooks.py", line 513 in __call__
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/_pytest/main.py", line 362 in pytest_runtestloop
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/pluggy/_callers.py", line 103 in _multicall
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/pluggy/_manager.py", line 120 in _hookexec
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/pluggy/_hooks.py", line 513 in __call__
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/_pytest/main.py", line 337 in _main
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/_pytest/main.py", line 283 in wrap_session
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/_pytest/main.py", line 330 in pytest_cmdline_main
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/pluggy/_callers.py", line 103 in _multicall
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/pluggy/_manager.py", line 120 in _hookexec
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/pluggy/_hooks.py", line 513 in __call__
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/_pytest/config/__init__.py", line 175 in main
  File "/Users/ash/code/airflow/airflow/.venv/lib/python3.12/site-packages/_pytest/config/__init__.py", line 201 in console_main
  File "/Users/ash/code/airflow/airflow/.venv/bin/pytest", line 8 in <module>

Extension modules: _time_machine, charset_normalizer.md, requests.packages.charset_normalizer.md, requests.packages.chardet.md, sqlalchemy.cimmutabledict, sqlalchemy.cprocessors, sqlalchemy.cresultproxy, yaml._yaml, _cffi_backend, lazy_object_proxy.cext, markupsafe._speedups, psutil._psutil_osx, psutil._psutil_posix, msgspec._core, psycopg2._psycopg, multidict._multidict, yarl._quoting_c, propcache._helpers_c, aiohttp._helpers, aiohttp._http_writer, aiohttp._http_parser, aiohttp._websocket, frozenlist._frozenlist, setproctitle._setproctitle (total: 24)
FAILED

One such error

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying with uv pip install git+https://github.com/dvarrazzo/py-setproctitle and I get the exact same behaviour fwiw.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, I am the setproctitle author, although I have limited experience with macOS and no hardware to test with it.

@gershnik has been so kind to contribute expertise and bugfixes to the project in relation with macOS. Maybe he might be able to help?

I wonder if this problem relates to concurrency during initialisation?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dvarrazzo I will definitely take a look after the holidays

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gershnik that's very appreciated. I wish you good holidays!

ashb
ashb reviewed Dec 20, 2024
View reviewed changes
task_sdk/src/airflow/sdk/execution_time/task_runner.py
# setproctitle causes issue on Mac OS: https://github.com/benoitc/gunicorn/issues/3021
os_type = sys.platform
if os_type == "darwin":
log.info("Mac OS detected, skipping setproctitle")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
log.info("Mac OS detected, skipping setproctitle")
log.debug("Mac OS detected, skipping setproctitle")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gershnik gershnik gershnik left review comments

@jlaneve jlaneve jlaneve left review comments

@potiuk potiuk potiuk left review comments

@dvarrazzo dvarrazzo dvarrazzo left review comments

@ashb ashb ashb approved these changes

@jedcunningham jedcunningham Awaiting requested review from jedcunningham jedcunningham will be requested when the pull request is marked ready for review jedcunningham is a code owner

@ephraimbuddy ephraimbuddy Awaiting requested review from ephraimbuddy ephraimbuddy will be requested when the pull request is marked ready for review ephraimbuddy is a code owner

Assignees
No one assigned
Labels
area:task-sdk
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jaketf @ashb @dvarrazzo @potiuk @jlaneve @gershnik